We don't rely on util in #3685 anymore, so we can also remove polyfills now with modern-node-polyfills package

The name of this plugin now doesn't seem appropriate, any naming ideas?

vitest:browser:tests

As far as I can tell util is still included by @sinonjs/fake-timers:

vitest/packages/vitest/src/integrations/mock/timers.ts

Line 38 in c8c1b4a

this._fakeTimers = withGlobal(global)

1. util is included dynamically if running inside Node.js
2. We bundle @sinonjs/fake-timers

Heres the error in the browser if we don't include util pollyfil

__vite-browser-external:util:3 Uncaught (in promise) Error: Module "util" has been externalized for browser compatibility. Cannot access "util.default" in client code.  See http://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.
    at Object.get (__vite-browser-external:util:3:11)
    at withGlobal (vendor-vi.f4f5ef45.js:1370:47)
    at vendor-vi.f4f5ef45.js:3017:32
    at vendor-vi.f4f5ef45.js:3023:3

Probably because we have process mocked, which we also shouldn't do, but it should be addressed in a separate issue.

Probably because we have process mocked, which we also shouldn't do, but it should be addressed in a separate issue.

Something like this #3701?

Probably because we have process mocked, which we also shouldn't do, but it should be addressed in a separate issue.

Something like this #3701?

Yes

Heres the error in the browser if we don't include util pollyfil

Why are tests passing then 🤔

OK, the tests will fail until #3701 is in

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Name	Link
🔨 Latest commit	0f333f8
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/650d90ed8063f40008426c62
😎 Deploy Preview	https://deploy-preview-3648--fastidious-cascaron-4ded94.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

normalizeId is not used anymore

Very true! Removed ✅

@sheremet-va are the tests generally a bit flakey? It seems to be a different spec failing each time

@sheremet-va are the tests generally a bit flakey? It seems to be a different spec failing each time

Some tests are flaky. Your PR is open until we have a browser test in ecosystem-ci

Some tests are flaky. Your PR is open until we have a browser test in ecosystem-ci

@sheremet-va what is the status of that? is it something I can help with?

Some tests are flaky. Your PR is open until we have a browser test in ecosystem-ci

@sheremet-va what is the status of that? is it something I can help with?

Sorry for keeping you waiting! I am working on a small test suite for ecosystem-ci now, we can probably merge this PR today or next week.

How should the util be polyfilled now? It's still used by @sinonjs/fake-timers. v1-beta fails with error SyntaxError: The requested module '/node_modules/.pnpm/util@0.12.5/node_modules/util/util.js?v=c8f99d41' does not provide an export named '__vi_inject__'. And when adding "vitest > util" to optimizeDeps.include, it fails with:

ReferenceError: process is not defined
 ❯ node_modules/.pnpm/util@0.12.5/node_modules/util/util.js ../../../../node_modules/.vite/deps/vitest___util.js:1308:5
 ❯ __require ../../../../node_modules/.vite/deps/chunk-UXIASGQL.js:9:50
 ❯ ../../../../node_modules/.vite/deps/vitest___util.js:1799:25

How should the util be polyfilled now?

Vitest doesn't use it and as far as I know @sinonjs/fake-timers checks it at runtime, so this shouldn't be a problem? If it is, maybe we can patch it and inline the package?

And when adding "vitest > util" to optimizeDeps.include, it fails with:

The error looks correct to me since we don't define a global process variable since it doesn't exist in the browser.

Vitest doesn't use it

Looks like @vitest/browser imports vitest/runners, which uses vi. Importing vi brings in inlined fake-timers which has process. The vendor-vi has import require$$2 from 'util';.

Vitest doesn't use it

Looks like @vitest/browser imports vitest/runners, which uses vi. Importing vi brings in inlined fake-timers which has process. The vendor-vi has import require$$2 from 'util';.

fake-timers have a check for process. What I assume happened in your case is that polyfill is using process for some reason.

The vendor-vi has import require$$2 from 'util';.

Yeah, this is why I am saying we should just patch fake timers to remove this check.